-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
rust: remove unused #9256
rust: remove unused #9256
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #9256 +/- ##
=======================================
Coverage 82.42% 82.43%
=======================================
Files 968 968
Lines 273952 273914 -38
=======================================
- Hits 225807 225790 -17
+ Misses 48145 48124 -21
Flags with carried forward coverage won't be shown. Click here to find out more. |
WARNING: ERROR: QA failed on SURI_TLPR1_alerts_cmp.
Pipeline 15309 |
@@ -103,29 +101,6 @@ impl ConfNode { | |||
return Self { conf } | |||
} | |||
|
|||
pub fn get_child_value(&self, key: &str) -> Option<&str> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I consider this part of the public api footprint.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
@@ -373,36 +373,6 @@ impl JsonBuilder { | |||
} | |||
} | |||
|
|||
/// Add a byte array to a JSON array encoded as hex. | |||
pub fn append_hex(&mut self, val: &[u8]) -> Result<&mut Self, JsonError> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can go, I see it coming back at some point tho. It kind of falls in the public API space, but we don't have plugin support for protos so is largely not usable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok
Side note that for plugin support for photos (for which I have a POC), you cannot export rust.
So, my rust plugin uses the C API of json builder...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Side note that for plugin support for photos (for which I have a POC), you cannot export rust. So, my rust plugin uses the C API of json builder...
Yeah, thats a known unfortunately. Rust plugins have to link in with the C api. We can provide Rust wrappers to make it feel more like Rust tho.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But Rust plugins can use standalone modules provided in Suricata proper like JSONBuilder, etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But Rust plugins can use standalone modules provided in Suricata proper like JSONBuilder
How so ?
my rust plugin uses the C API of jsonbuilder...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let keep the conf API item.
Replaced by #9419 |
Link to redmine ticket:
None, generic cleaning
https://redmine.openinfosecfoundation.org/issues/4083
Describe changes:
Found with
git grep 'pub ' rust/src/ | cut -d: -f1 | uniq | xargs sed -i -e 's/pub /pub(crate) /'
then see rust warnings when compilingThere is nothing more to be done :
rrclass
in DNSQueryEntry) : I feel good to keep themRebase #9020, now that Suricata7 was released